-
Notifications
You must be signed in to change notification settings - Fork 537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
implement yyset_interactive #291
base: master
Are you sure you want to change the base?
Conversation
src/flex.skl
Outdated
@@ -618,6 +619,7 @@ void yyfree ( void * M4_YY_PROTO_LAST_ARG ); | |||
|
|||
m4_ifdef( [[M4_YY_NOT_IN_HEADER]], | |||
[[ | |||
|
|||
#define yy_new_buffer yy_create_buffer | |||
#define yy_set_interactive(is_interactive) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you are implementing yyset_interactive, could you also remove this yy_set_interactive macro, which could cause confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Explorer09 Good call, I will get this updated tonight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Explorer09 This is done.
Sorry for my ignorance: Why not simply amending the definition of the macro [EDIT] ... like so? --- C:/myDevel/myprogs/flex/flex_2.6.4/yy_interactive/flex1.skl Wed Aug 23 13:00:27 2017
+++ C:/myDevel/myprogs/flex/flex_2.6.4/yy_interactive/flex.skl Tue Dec 12 17:19:38 2017
@@ -611,8 +611,9 @@
m4_ifdef( [[M4_YY_NOT_IN_HEADER]],
[[
#define yy_new_buffer yy_create_buffer
-#define yy_set_interactive(is_interactive) \
+#define yy_set_interactive(is_interactive M4_YY_CALL_LAST_ARG) \
{ \
+ M4_YY_DECL_GUTS_VAR(); \
if ( ! YY_CURRENT_BUFFER ){ \
yyensure_buffer_stack (M4_YY_CALL_ONLY_ARG); \
YY_CURRENT_BUFFER_LVALUE = \ Are there any issues when doing so? Of course, I see the advantage of having a function instead of a macro, especially from another c file, however, macros are preferred by flex. Separately, would it be fair to say that the remainder of the changes are enhancements to command line options only? |
@jannick0 The use of the function instead of a macro is because YY_CURRENT_BUFFER_LVALUE is an internal macro, AFAIK. That won't be the problem if yy_set_interactive is called only within the .l file, but that's not the case. |
@Explorer09 Agree, but the issue then becomes a philosophical one, namely if the implementation should prefer macros (as is essentially right now) or functions. Using a wrapper would a way out of the current situation to effectively call macro defined in the lex file only without any changes. The short patch above is amending what I would consider a bug, since for reentrant scanners the opaque scanner object needs to be resolved as it contains the flag |
@jannick0 If you read #290, you can see that |
There are weaknesses of C macro mainly because they work like simple string substitution. If you only use C macro for reasons of function inlining, please don't, or do it sparingly. I should have mentioned that yy_set_interactive, in it's current macro form, lack the proper |
Again, using macros or functions is a question of coding style which is not my business here, supposedly rather the package maintainer's. Other issues about how to use macros are interesting, but to be dealt with in a different place I would say. Of course, I agree that they should certainly be followed for implemented macros. If a function implementation of |
Could you rebase and resolve conflicts? As you can see, we're (finally) clearing out the backlog. |
Currently, only available in reentrant mode, since it needs a scanner argument. However, I believe a
yyinteractive
macro can be implemented too. References #290